fix: add network error retry for transfer polling #763
Conversation
|
from the Logs |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| consecutiveErrors++ | ||
| Logger.warn( | ||
| "Failed to fetch order '$orderId' (attempt $consecutiveErrors/$MAX_CONSECUTIVE_ERRORS)", | ||
| e = it, |
There was a problem hiding this comment.
According to CLAUDE.md:
NEVER add
e =named parameter to Logger calls
The Logger.warn call should pass the throwable without the named parameter syntax.
| e = it, | |
| Logger.warn( | |
| "Failed to fetch order '$orderId' (attempt $consecutiveErrors/$MAX_CONSECUTIVE_ERRORS)", | |
| it, | |
| context = TAG | |
| ) |
| @Test | ||
| fun `refreshOrders returns failure when server throws`() = test { | ||
| sut = createSut() | ||
| wheneverBlocking { coreService.blocktank.orders(refresh = true) }.thenThrow(RuntimeException("Network error")) |
There was a problem hiding this comment.
According to CLAUDE.md:
NEVER use
wheneverBlockingin unit test expression body functions wrapped in a= test {}lambda
ALWAYS wrap unit testssetUpmethods mocking suspending calls withrunBlocking, e.gsetUp() = runBlocking {}
Move the mock setup to the setUp method using runBlocking instead of using wheneverBlocking inside the test expression body.
| @Test | ||
| fun `getOrder returns failure when refresh fails`() = test { | ||
| sut = createSut() | ||
| wheneverBlocking { coreService.blocktank.orders(refresh = true) }.thenThrow(RuntimeException("Network error")) |
There was a problem hiding this comment.
According to CLAUDE.md:
NEVER use
wheneverBlockingin unit test expression body functions wrapped in a= test {}lambda
ALWAYS wrap unit testssetUpmethods mocking suspending calls withrunBlocking, e.gsetUp() = runBlocking {}
Move the mock setup to the setUp method using runBlocking instead of using wheneverBlocking inside the test expression body.
| consecutiveErrors++ | ||
| Logger.warn( | ||
| "Failed to fetch order '$orderId' (attempt $consecutiveErrors/$MAX_CONSECUTIVE_ERRORS)", | ||
| e = it, |
There was a problem hiding this comment.
The named parameter e = should not be used in Logger calls.
CLAUDE.md Rule:
NEVER add
e =named parameter to Logger calls
Reference: CLAUDE.md:180
Fix:
| e = it, | |
| it, |
| @Test | ||
| fun `refreshOrders returns failure when server throws`() = test { | ||
| sut = createSut() | ||
| wheneverBlocking { coreService.blocktank.orders(refresh = true) }.thenThrow(RuntimeException("Network error")) |
There was a problem hiding this comment.
The wheneverBlocking call should not be used inside = test {} expression body functions.
CLAUDE.md Rule:
NEVER use
wheneverBlockingin unit test expression body functions wrapped in a= test {}lambdaALWAYS wrap unit tests
setUpmethods mocking suspending calls withrunBlocking, e.gsetUp() = runBlocking {}
Reference: CLAUDE.md:205-206
Suggested Fix:
Move the wheneverBlocking mock setup to the setUp() method wrapped in runBlocking, as shown in the existing tests in this file (see lines 42-49). Both new test methods (refreshOrders returns failure when server throws and getOrder returns failure when refresh fails) need this change.
Example pattern from existing tests:
@Before
fun setUp() = runBlocking {
wheneverBlocking { coreService.blocktank.orders(refresh = true) }.thenThrow(RuntimeException("Network error"))
}
|
draft to apply suggestions |
1 participant
Fixes #608
This PR adds network error resilience to the transfer-to-spending polling mechanism by distinguishing between transient network failures and permanent order errors.
Description
.fold()instead of.getOrNull()to properly handleResultfailuresPreview
Screen_recording_20260206_132910.mp4
QA Notes
To facilitate the simulation, you can put a breakpoint in the line and then disable internet
1. Normal transfer flow (regression)
2. Network error recovery
3. Persistent network failure